Source/include/Ably/ARTPaginatedResult.h (1)

30-31: Approved: Enables mocking as intended, but consider additional safeguards

The changes to the init method and its associated comment effectively achieve the PR's objective of allowing mocking of ARTPaginatedResult. This improvement in testability is valuable for the project.

However, there's a potential risk of misuse if developers instantiate ARTPaginatedResult directly without subclassing. To mitigate this:

1. Consider adding a compile-time warning or runtime assertion in the init method to further discourage direct usage.
2. Update the class documentation to clearly state that this class is intended for subclassing, not direct instantiation.

Would you like assistance in implementing these additional safeguards?

Source/ARTPaginatedResult.m (1)

35-41: Consider marking init as a designated initializer

Since init is now publicly available and sets _initializedViaInit, consider explicitly marking it as a designated initializer. This enhances clarity and ensures proper initialization in subclasses.

Source/ARTPaginatedResult.m (1)

88-116: 🛠️ Refactor suggestion

Refactor repeated exception handling into a helper method

The checks for _initializedViaInit and the associated exceptions are repeated in the methods hasNext, isLast, items, first, and next. To improve maintainability and reduce code duplication, consider extracting this logic into a helper method.

Apply this diff to create a helper method and utilize it in the relevant methods:

+ - (void)assertInitializedViaSubclass {
+     if (_initializedViaInit) {
+         [NSException raise:NSInvalidArgumentException format:@"When initializing this class using -init, you need to override this method in a subclass"];
+     }
+ }

- (BOOL)hasNext {
-     if (_initializedViaInit) {
-         [NSException raise:NSInvalidArgumentException format:@"When initializing this class using -init, you need to override this method in a subclass"];
-     }
+     [self assertInitializedViaSubclass];
      return _hasNext;
}

- (BOOL)isLast {
-     if (_initializedViaInit) {
-         [NSException raise:NSInvalidArgumentException format:@"When initializing this class using -init, you need to override this method in a subclass"];
-     }
+     [self assertInitializedViaSubclass];
      return _isLast;
}

- (NSArray<id> *)items {
-     if (_initializedViaInit) {
-         [NSException raise:NSInvalidArgumentException format:@"When initializing this class using -init, you need to override this method in a subclass"];
-     }
+     [self assertInitializedViaSubclass];
      return _items;
}

- (void)first:(void (^)(ARTPaginatedResult<id> *_Nullable result, ARTErrorInfo *_Nullable error))callback {
-     if (_initializedViaInit) {
-         [NSException raise:NSInvalidArgumentException format:@"When initializing this class using -init, you need to override this method in a subclass"];
-     }
+     [self assertInitializedViaSubclass];
     // ... rest of the method ...
}

- (void)next:(void (^)(ARTPaginatedResult<id> *_Nullable result, ARTErrorInfo *_Nullable error))callback {
-     if (_initializedViaInit) {
-         [NSException raise:NSInvalidArgumentException format:@"When initializing this class using -init, you need to override this method in a subclass"];
-     }
+     [self assertInitializedViaSubclass];
     // ... rest of the method ...
}

Also applies to: 130-133

Source/include/Ably/ARTPaginatedResult.h (1)

Line range hint 1-14: Update class documentation to reflect new initialization behavior

With the changes to the init method, it's important to update the class documentation to provide clear guidance on usage, especially regarding subclassing and thread safety. Consider adding the following information to the class documentation:

1. Explain that this class is designed for subclassing when used in production code.
2. Clarify the intended use of the public initializer for mocking in test scenarios.
3. Provide guidance on how to properly subclass ARTPaginatedResult for production use.
4. Ensure that the NS_SWIFT_SENDABLE attribute is still appropriate with the new initialization behavior, and document any thread-safety considerations.

Here's a suggested addition to the class documentation:

/**
 * Contains a page of results for message or presence history, stats, or REST presence requests. An `ARTPaginatedResult` response from a REST API paginated query is also accompanied by metadata that indicates the relative queries available to the `ARTPaginatedResult` object.
 *
 * This class is designed to be subclassed for production use. The public initializer is provided primarily for creating mock instances in test scenarios. When subclassing, ensure to override and implement all required methods and properties.
 *
 * Thread Safety: This class is marked as `NS_SWIFT_SENDABLE`. Ensure that any subclass maintains thread safety, especially when overriding methods or properties.
 */

This addition provides crucial information for developers working with this class, reducing the likelihood of misuse and clarifying its intended usage patterns.

Configuration used: CodeRabbit UI
Review profile: CHILL

Source/ARTPaginatedResult.m (1)

87-88: Use appropriate exception type and include method name in exception message

The method initializedViaInitCheck currently raises an NSInvalidArgumentException with a fixed message. It's more appropriate to use NSInternalInconsistencyException, and including the method name in the exception message will provide more clarity about which method needs to be overridden.

Apply this diff to improve the exception:

-        [NSException raise:NSInvalidArgumentException format:@"When initializing this class using -init, you need to override this method in a subclass"];
+        [NSException raise:NSInternalInconsistencyException format:@"You must override %@ in a subclass when initializing this class using -init", NSStringFromSelector(_cmd)];

Configuration used: CodeRabbit UI
Review profile: CHILL

Source/include/Ably/ARTPaginatedResult.h (1)

29-30: Good improvements, consider additional enhancements for clarity and safety

The changes align well with the PR objective of enabling mocking for ARTPaginatedResult. The added documentation clearly explains the limitations and intended use, and the use of NS_DESIGNATED_INITIALIZER is a good improvement. However, to further enhance the API's clarity and safety, consider the following suggestions:

1. Use the NS_SWIFT_UNAVAILABLE macro to prevent unintended usage in Swift:

- (instancetype)init NS_DESIGNATED_INITIALIZER NS_SWIFT_UNAVAILABLE("Use a subclass for creating mock instances in tests");

2. Add a class method for creating mock instances, making the testing purpose more explicit:

+ (instancetype)mockForTesting;

These changes would provide a clearer API for both production and testing use cases while maintaining the desired flexibility for mocking and adding an extra layer of safety.

Configuration used: CodeRabbit UI
Review profile: CHILL